Improve a couple of regex source generator oddities #103851
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When we can't efficiently enumerate a character class and it's not just a single range, we fall back to outputting a helper IndexOfXx method. This method does an optimized search for either any ASCII member of the set or anything non-ASCII, and then falls back to walking the items one-by-one. This makes sense, unless all of ASCII is in the set, in which case this is a meaningless IndexOfAny call because it's just going to always return 0 or -1. We should avoid emitting an IndexOfAny call in such a case.
… the full ASCII range. The intent was for this to literally output the text `IndexOfAnyExceptInRange('\0', '\u007f')` into the generated C#, but because those are single slashes, it's actually outputting the characters (char)0x0 and (char(0x7F. That's functionally correct, but it's not what was intended and makes the code harder to read. The fix is just to put in the missing slashes.
|
Tagging subscribers to this area: @dotnet/area-system-text-regularexpressions |
This was referenced Jun 22, 2024
Open
MihaZupan
approved these changes
Jun 22, 2024
rzikm
pushed a commit
to rzikm/dotnet-runtime
that referenced
this pull request
Jun 24, 2024
* Avoiding generating a useless IndexOfAny
When we can't efficiently enumerate a character class and it's not just a single range, we fall back to outputting a helper IndexOfXx method. This method does an optimized search for either any ASCII member of the set or anything non-ASCII, and then falls back to walking the items one-by-one. This makes sense, unless all of ASCII is in the set, in which case this is a meaningless IndexOfAny call because it's just going to always return 0 or -1. We should avoid emitting an IndexOfAny call in such a case.
* This helper is emitting a call to IndexOfAnyExceptInRange, passing in the full ASCII range. The intent was for this to literally output the text `IndexOfAnyExceptInRange('\0', '\u007f')` into the generated C#, but because those are single slashes, it's actually outputting the characters (char)0x0 and (char(0x7F. That's functionally correct, but it's not what was intended and makes the code harder to read. The fix is just to put in the missing slashes.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
IndexOfAnyExceptInRange('\0', '\u007f')into the generated C#, but because those are single slashes, it's actually outputting the characters (char)0x0 and (char(0x7F. That's functionally correct, but it's not what was intended and makes the code harder to read. The fix is just to put in the missing slashes.Closes #103842
Closes #103840